{Packaging} Fix cask template and az upgrade for preview and homebrew-cask migration#33168
Open
naga-nandyala wants to merge 3 commits intoAzure:devfrom
Open
{Packaging} Fix cask template and az upgrade for preview and homebrew-cask migration#33168naga-nandyala wants to merge 3 commits intoAzure:devfrom
az upgrade for preview and homebrew-cask migration#33168naga-nandyala wants to merge 3 commits intoAzure:devfrom
Conversation
…oduction During the preview phase (Plan B), the cask is distributed as `azure-cli-preview` via a custom tap, while post-migration it becomes `azure-cli` in homebrew-cask. The `az upgrade` command now checks which cask token is installed and upgrades accordingly.
naga-nandyala
requested review from
bebound,
jiasli and
wangzelin007
as code owners
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
Collaborator
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
Contributor
There was a problem hiding this comment.
Pull request overview
Updates az upgrade (util command module) to correctly upgrade Azure CLI when installed via Homebrew cask during the preview-to-production transition, by selecting the installed cask token before running brew upgrade.
Changes:
- Detect whether
azure-cli-previewis installed via Homebrew cask and select the appropriate cask token. - Use the detected cask token when invoking
brew upgrade --cask ...forHOMEBREW_CASKinstalls.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…x completion ordering
naga-nandyala
changed the title
az upgrade: Detect correct cask token for preview vs productionaz upgrade for homebrew-cask migration (Plan B)
…d narrower exception handling Address Copilot review feedback: - Check both azure-cli-preview and azure-cli cask tokens explicitly - Fail early with UPGRADE_MSG if neither cask is installed - Catch OSError instead of broad Exception, with debug logging
naga-nandyala
changed the title
az upgrade for homebrew-cask migration (Plan B)az upgrade for preview and homebrew-cask migration
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+135
to
154
| # Determine cask token: 'azure-cli-preview' (custom tap) or 'azure-cli' (homebrew-cask) | ||
| try: | ||
| preview_installed = subprocess.call(['brew', 'list', '--cask', 'azure-cli-preview'], | ||
| stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) == 0 | ||
| cli_installed = subprocess.call(['brew', 'list', '--cask', 'azure-cli'], | ||
| stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) == 0 | ||
| if preview_installed: | ||
| cask_token = 'azure-cli-preview' | ||
| elif cli_installed: | ||
| cask_token = 'azure-cli' | ||
| else: | ||
| raise CLIError(UPGRADE_MSG) | ||
| except OSError as ex: | ||
| logger.debug("Failed to detect Homebrew cask token: %s", ex) | ||
| cask_token = 'azure-cli' | ||
| exit_code = subprocess.call(['brew', 'update']) | ||
| if exit_code == 0: | ||
| update_cmd = ['brew', 'upgrade', '--cask', 'azure-cli'] | ||
| update_cmd = ['brew', 'upgrade', '--cask', cask_token] | ||
| logger.debug("Update azure cli with '%s'", " ".join(update_cmd)) | ||
| exit_code = subprocess.call(update_cmd) |
There was a problem hiding this comment.
The new HOMEBREW_CASK token-detection logic (switching between azure-cli-preview and azure-cli) isn’t covered by tests. Since this path changes which package gets upgraded and can raise CLIError, it would be good to add unit coverage that mocks subprocess.call to validate selection and the failure case (neither cask installed).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two packaging fixes for the upcoming homebrew-cask migration (Plan B):
1.
az upgrade: Detect correct cask token for preview vs productionThe current
az upgradehardcodesbrew upgrade --cask azure-cli. Under Plan B, preview users install via theazure-cli-previewcask token (from custom tapAzure/homebrew-azure-cli). This fix adds runtime detection soaz upgradeuses the correct token.Change:
src/azure-cli/azure/cli/command_modules/util/custom.pyazure-cli-previewcask is installed viabrew list --cask azure-cli-previewazure-cli-previewas the cask token; otherwise default toazure-cli2. Fix cask definition template (
azure-cli.rb.in)Fixes discovered during Homebrew PR #257931:
Change:
scripts/release/macos/templates/azure-cli.rb.inos macos: "macos"stanza — caused Homebrew CI to simulate Linux tags, resulting insha256: nilaudit failuremacosin URL — replaced#{os}interpolation sinceosstanza is removedverified:parameter to URL — required by Homebrew when URL domain differs from homepage domainThe updated template now matches the working cask in the custom tap
Azure/homebrew-azure-cli.Testing
azure-cli-previewcask inAzure/homebrew-azure-cliaz upgradelogic tested with bothazure-cliandazure-cli-previewcask tokens